Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix unarmed weapon experience gain #62578

Merged
merged 4 commits into from
Jan 16, 2023
Merged

Fix unarmed weapon experience gain #62578

merged 4 commits into from
Jan 16, 2023

Conversation

robob27
Copy link
Contributor

@robob27 robob27 commented Dec 5, 2022 •
edited
Loading

Summary

Bugfixes "Using worn unarmed weapons raises unarmed skill"

Purpose of change

Resolves #61682

Note: If you are a mod creator and see this, please suggest changes to the PR to match the specific values you'd like to see on your items. At the time of writing this, the values are extremely WIP and I plan to make corrections to what is there. But if you have the exact values you'd like to see there, please let me know and I'll fix it up.

Describe the solution

Since the attack vector changes made in #53954 using unarmed weapons has not been properly training unarmed skills. This is because those changes removed a change added in #41209 which added a temporary UNARMED_WEAPON flag to items covering your hands when not wielding a weapon.

This first commit of the PR removes all usage of the UNARMED_WEAPON flag in c++, instead checking for arms where needed. This should fix all non mod related items to train unarmed properly.

The second commit removes all uses of the UNARMED_WEAPON flag in JSON, including all mods. Potentially includes some undesirable changes to mod items unless feedback is received for all items, but maybe better than stable with unarmed weapons not training properly.

The third commit removes the flag entirely, probably breaking some third party mods, but this sounds okay based on some discussion in the stable push channel.

Describe alternatives you've considered

Adding back UNARMED_WEAPON to all the unarmed weapons but I don't think that's the move.

Testing

WIP

Additional context

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Martial Arts Arts, Techniques, weapons and anything touching martial arts. Melee Melee weapons, tactics, techniques, reach attack NPC / Factions NPCs, AI, Speech, Factions, Ownership <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Dec 5, 2022
@robob27 robob27 force-pushed the unarmed-training-fix branch from 93cb01f to bcc77cd Compare December 5, 2022 08:00
@github-actions github-actions bot added BasicBuildPassed This PR builds correctly, label assigned by github actions Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves Items: Armor / Clothing Armor and clothing Mods Issues related to mods or modding Mods: Aftershock Anything to do with the Aftershock mod Mods: Magiclysm Anything to do with the Magiclysm mod labels Dec 5, 2022
github-actions[bot]
github-actions bot reviewed Dec 5, 2022
View reviewed changes
data/mods/Magiclysm/items/enchanted_unarmed.json Outdated Show resolved Hide resolved
data/mods/Magiclysm/items/enchanted_unarmed.json Outdated Show resolved Hide resolved
data/mods/Magiclysm/items/enchanted_unarmed.json Outdated Show resolved Hide resolved
data/mods/Magiclysm/items/enchanted_unarmed.json Outdated Show resolved Hide resolved
@github-actions github-actions bot removed json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Dec 5, 2022
@robob27 robob27 force-pushed the unarmed-training-fix branch from 5803f3e to 988fa59 Compare December 5, 2022 23:33
@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Dec 5, 2022
@Maleclypse
Copy link
Member

Is this intended to still be in draft or is it ready for review?

@ghost
Copy link

ghost commented Dec 10, 2022

Seems to be working properly with a cursory inspection. Checked that unarmed skill is increasing when no weapon is worn and checked that both the weapon's damage type and unarmed skill is gaining xp when wielding a set of skewer knuckles.

@robob27
Copy link
Contributor Author

robob27 commented Dec 10, 2022 via email

@ghost
Copy link

ghost commented Dec 20, 2022 •
edited by ghost
Loading

I'm now getting some errors that may be be related to this fix. The character in question has a set of skewer knuckles in their inventory, but is not wearing them. He also has less than 1 unarmed skill. I noticed the errors sometime after taking the skewers off. These messages occur during long activities, sleep being the easiest to trigger. They may be occurring more often.

00:19:21.736 ERROR : src/item.cpp:7284 [bool item::has_flag(const flag_id &, bool) const] Attempted to check invalid flag_id UNARMED_WEAPON
00:19:47.469 ERROR : (error message will follow backtrace)
./cataclysm-tiles(debug_write_backtrace(std::ostream&)+0x25) [0x152ba2a]
./cataclysm-tiles(DebugLog(DebugLevel, DebugClass)+0x278) [0x152afb8]
./cataclysm-tiles(realDebugmsg(char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)+0xa0) [0x152a892]
./cataclysm-tiles(void realDebugmsg<std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&>(char const*, char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)+0x5f) [0x1146f53]
./cataclysm-tiles(item::has_flag(string_id<json_flag> const&, bool) const+0x7f) [0x181c093]
./cataclysm-tiles(item::melee_skill() const+0x13) [0x185d9fb]
./cataclysm-tiles(Character::get_hit_weapon(item const&) const+0x16) [0x1bcad4c]
./cataclysm-tiles(item::effective_dps(Character const&, Creature&) const+0x4d) [0x182aa91]
./cataclysm-tiles(item::dps[abi:cxx11](bool, bool, Character const&) const+0x8f) [0x182b3af]
./cataclysm-tiles(item::melee_combat_info(std::vector<iteminfo, std::allocator >&, iteminfo_query const*, int, bool) const+0x42c) [0x1853588]
./cataclysm-tiles(item::info[abi:cxx11](std::vector<iteminfo, std::allocator >&, iteminfo_query const*, int) const+0x2e4) [0x1829bb8]
./cataclysm-tiles(item::info[abi:cxx11](bool, std::vector<iteminfo, std::allocator >&) const+0x28) [0x18298a6]
./cataclysm-tiles(inventory_selector::action_examine(item_location const&)+0x51) [0x180591d]
./cataclysm-tiles(inventory_pick_selector::execute()+0x89) [0x18069d9]
./cataclysm-tiles() [0x172367d]
./cataclysm-tiles(game_menus::inv::gun_to_modify(Character&, item const&)+0xce) [0x1728ff9]
./cataclysm-tiles(iuse::gunmod_attach(Character*, item*, bool, tripoint const&)+0x62) [0x1957413]
./cataclysm-tiles(iuse_function_wrapper::use(Character&, item&, bool, tripoint const&) const+0x16) [0x18ed400]
./cataclysm-tiles(use_function::call(Character&, item&, bool, tripoint const&) const+0xd) [0x195cd7b]
./cataclysm-tiles(itype::invoke(Character&, item&, tripoint const&, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&) const+0x92) [0x1929b46]

Attempting to repeat stack trace using debug symbols…
debug_write_backtrace(std::ostream&)
??:?
DebugLog(DebugLevel, DebugClass)
??:?
realDebugmsg(char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
??:?
void realDebugmsg<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(char const*, char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
??:?
item::has_flag(string_id<json_flag> const&, bool) const
??:?
item::melee_skill() const
??:?
Character::get_hit_weapon(item const&) const
??:?
item::effective_dps(Character const&, Creature&) const
??:?
item::dps[abi:cxx11](bool, bool, Character const&) const
??:?
item::melee_combat_info(std::vector<iteminfo, std::allocator<iteminfo> >&, iteminfo_query const*, int, bool) const
??:?
item::info[abi:cxx11](std::vector<iteminfo, std::allocator<iteminfo> >&, iteminfo_query const*, int) const
??:?
item::info[abi:cxx11](bool, std::vector<iteminfo, std::allocator<iteminfo> >&) const
??:?
inventory_selector::action_examine(item_location const&)
??:?
inventory_pick_selector::execute()
??:?
inv_internal(Character&, inventory_selector_preset const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, item_location)
game_inventory.cpp:?
game_menus::inv::gun_to_modify(Character&, item const&)
??:?
iuse::gunmod_attach(Character*, item*, bool, tripoint const&)
??:?
iuse_function_wrapper::use(Character&, item&, bool, tripoint const&) const
??:?
use_function::call(Character&, item&, bool, tripoint const&) const
??:?
itype::invoke(Character&, item&, tripoint const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const
??:?

Backtrace emission took 1 seconds.
(continued from above) ERROR : src/item.cpp:7284 [bool item::has_flag(const flag_id &, bool) const] Attempted to check invalid flag_id UNARMED_WEAPON
[ Previous repeated 17 times ]

@Maleclypse
Copy link
Member

I'm now getting some errors that may be be related to this fix. The character in question has a set of skewer knuckles in their inventory, but is not wearing them. He also has less than 1 unarmed skill. I noticed the errors sometime after taking the skewers off. These messages occur during long activities, sleep being the easiest to trigger. They may be occurring more often.

00:19:21.736 ERROR : src/item.cpp:7284 [bool item::has_flag(const flag_id &, bool) const] Attempted to check invalid flag_id UNARMED_WEAPON 00:19:47.469 ERROR : (error message will follow backtrace) ./cataclysm-tiles(debug_write_backtrace(std::ostream&)+0x25) [0x152ba2a] ./cataclysm-tiles(DebugLog(DebugLevel, DebugClass)+0x278) [0x152afb8] ./cataclysm-tiles(realDebugmsg(char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)+0xa0) [0x152a892] ./cataclysm-tiles(void realDebugmsg<std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&>(char const*, char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)+0x5f) [0x1146f53] ./cataclysm-tiles(item::has_flag(string_id<json_flag> const&, bool) const+0x7f) [0x181c093] ./cataclysm-tiles(item::melee_skill() const+0x13) [0x185d9fb] ./cataclysm-tiles(Character::get_hit_weapon(item const&) const+0x16) [0x1bcad4c] ./cataclysm-tiles(item::effective_dps(Character const&, Creature&) const+0x4d) [0x182aa91] ./cataclysm-tiles(item::dps[abi:cxx11](bool, bool, Character const&) const+0x8f) [0x182b3af] ./cataclysm-tiles(item::melee_combat_info(std::vector<iteminfo, std::allocator >&, iteminfo_query const*, int, bool) const+0x42c) [0x1853588] ./cataclysm-tiles(item::info[abi:cxx11](std::vector<iteminfo, std::allocator >&, iteminfo_query const*, int) const+0x2e4) [0x1829bb8] ./cataclysm-tiles(item::info[abi:cxx11](bool, std::vector<iteminfo, std::allocator >&) const+0x28) [0x18298a6] ./cataclysm-tiles(inventory_selector::action_examine(item_location const&)+0x51) [0x180591d] ./cataclysm-tiles(inventory_pick_selector::execute()+0x89) [0x18069d9] ./cataclysm-tiles() [0x172367d] ./cataclysm-tiles(game_menus::inv::gun_to_modify(Character&, item const&)+0xce) [0x1728ff9] ./cataclysm-tiles(iuse::gunmod_attach(Character*, item*, bool, tripoint const&)+0x62) [0x1957413] ./cataclysm-tiles(iuse_function_wrapper::use(Character&, item&, bool, tripoint const&) const+0x16) [0x18ed400] ./cataclysm-tiles(use_function::call(Character&, item&, bool, tripoint const&) const+0xd) [0x195cd7b] ./cataclysm-tiles(itype::invoke(Character&, item&, tripoint const&, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&) const+0x92) [0x1929b46]

Attempting to repeat stack trace using debug symbols…
debug_write_backtrace(std::ostream&)
??:?
DebugLog(DebugLevel, DebugClass)
??:?
realDebugmsg(char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
??:?
void realDebugmsg<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(char const*, char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
??:?
item::has_flag(string_id<json_flag> const&, bool) const
??:?
item::melee_skill() const
??:?
Character::get_hit_weapon(item const&) const
??:?
item::effective_dps(Character const&, Creature&) const
??:?
item::dps[abi:cxx11](bool, bool, Character const&) const
??:?
item::melee_combat_info(std::vector<iteminfo, std::allocator<iteminfo> >&, iteminfo_query const*, int, bool) const
??:?
item::info[abi:cxx11](std::vector<iteminfo, std::allocator<iteminfo> >&, iteminfo_query const*, int) const
??:?
item::info[abi:cxx11](bool, std::vector<iteminfo, std::allocator<iteminfo> >&) const
??:?
inventory_selector::action_examine(item_location const&)
??:?
inventory_pick_selector::execute()
??:?
inv_internal(Character&, inventory_selector_preset const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, item_location)
game_inventory.cpp:?
game_menus::inv::gun_to_modify(Character&, item const&)
??:?
iuse::gunmod_attach(Character*, item*, bool, tripoint const&)
??:?
iuse_function_wrapper::use(Character&, item&, bool, tripoint const&) const
??:?
use_function::call(Character&, item&, bool, tripoint const&) const
??:?
itype::invoke(Character&, item&, tripoint const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const
??:?

Backtrace emission took 1 seconds. (continued from above) ERROR : src/item.cpp:7284 [bool item::has_flag(const flag_id &, bool) const] Attempted to check invalid flag_id UNARMED_WEAPON [ Previous repeated 17 times ]

So you are saying you are getting the errors when you loaded this fix into your world?

@Maleclypse
Copy link
Member

Any movement possible on this PR?

@Fris0uman
Copy link
Contributor

Could not trigger the skewer error with a new save or with an old save that got the skewer before the change

@Maleclypse Maleclypse marked this pull request as ready for review January 14, 2023 15:43
@ghost
Copy link

ghost commented Jan 14, 2023

So you are saying you are getting the errors when you loaded this fix into your world?

No. When adding this fix, it worked fine. Several days later, on a new character I began seeing the issue while sleeping, crafting etc. I think there may have been some updates to experimental in that time period. I haven't looked at this patch in some time as unarmed experience seems to be working in experimental for most of the weapons I've used recently. Not all, but most.

Scrap knuckles aren't training bash damage currently (before anyone asks).

@Fris0uman Fris0uman force-pushed the unarmed-training-fix branch from 2077b83 to b9ff317 Compare January 15, 2023 15:53
@kevingranade kevingranade merged commit 9024139 into CleverRaven:master Jan 16, 2023
@NetSysFire NetSysFire changed the title WIP Fix unarmed weapon experience gain Fix unarmed weapon experience gain Jan 16, 2023
@Maleclypse Maleclypse mentioned this pull request Jan 19, 2023
Closed
@MedievalYeetMachine MedievalYeetMachine mentioned this pull request Jan 23, 2023
Closed
@nemesis688 nemesis688 mentioned this pull request Mar 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@KorGgenT KorGgenT Awaiting requested review from KorGgenT

@Maleclypse Maleclypse Awaiting requested review from Maleclypse

@John-Candlebury John-Candlebury Awaiting requested review from John-Candlebury

@Mom-Bun Mom-Bun Awaiting requested review from Mom-Bun

Assignees
No one assigned
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves Items: Armor / Clothing Armor and clothing [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Martial Arts Arts, Techniques, weapons and anything touching martial arts. Melee Melee weapons, tactics, techniques, reach attack Mods: Aftershock Anything to do with the Aftershock mod Mods: Magiclysm Anything to do with the Magiclysm mod Mods Issues related to mods or modding NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unarmed Skill Learning Issues
4 participants
@robob27 @Maleclypse @Fris0uman @kevingranade